feat: enable web binary template by default in WebEncodePlugin#2545
Conversation
🦋 Changeset detectedLatest commit: 5290d99 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBinary template encoding is now the default for web templates; only ChangesWeb binary template defaulting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/enable-web-binary-template-by-default.md:
- Around line 18-20: The markdown link text has a missing space in
"`@lynx-js/web-core`Changelog"; update the link text to include a space so it
reads "`@lynx-js/web-core` Changelog" (locate and edit the text in the
.changeset/enable-web-binary-template-by-default.md diff block containing the
link).
In `@packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts`:
- Around line 108-111: The branch in WebEncodePlugin using
isExperimentalWebBinary includes an impossible numeric comparison
(isExperimentalWebBinary === 0); since process.env values are string|undefined,
remove the numeric comparison and only compare string values (e.g., 'false' and
'0') or handle undefined explicitly; update the conditional around
isExperimentalWebBinary in the WebEncodePlugin method to drop the === 0 check so
the unreachable clause is eliminated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be1ad1a1-bac5-475b-9677-2d837c743447
📒 Files selected for processing (4)
.changeset/enable-web-binary-template-by-default.mdpackages/web-platform/web-core-e2e/scripts/generate-build-command.jspackages/web-platform/web-tests/package.jsonpackages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts
💤 Files with no reviewable changes (1)
- packages/web-platform/web-core-e2e/scripts/generate-build-command.js
907e383 to
31a0800
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Merging this PR will improve performance by 18.85%
Performance Changes
Comparing Footnotes
|
React Example#7803 Bundle Size — 225.52KiB (0%).5290d99(current) vs 5f3b6eb main#7798(baseline) Bundle metrics
|
| Current #7803 |
Baseline #7798 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
180 |
180 |
|
69 |
69 |
|
44.54% |
44.54% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7803 |
Baseline #7798 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
79.77KiB |
79.77KiB |
Bundle analysis report Branch PupilTong:p/hw/opt-out-wasm Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9376 Bundle Size — 900.03KiB (0%).5290d99(current) vs 5f3b6eb main#9371(baseline) Bundle metrics
Bundle size by type
|
| Current #9376 |
Baseline #9371 |
|
|---|---|---|
495.9KiB |
495.9KiB |
|
401.92KiB |
401.92KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch PupilTong:p/hw/opt-out-wasm Project dashboard
Generated by RelativeCI Documentation Report issue
React External#918 Bundle Size — 680.82KiB (0%).5290d99(current) vs 5f3b6eb main#913(baseline) Bundle metrics
|
| Current #918 |
Baseline #913 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch PupilTong:p/hw/opt-out-wasm Project dashboard
Generated by RelativeCI Documentation Report issue
React Example (Element Template)#2 Bundle Size — 198.61KiB (0%).ef964b5(current) vs a317fd6 main#1(baseline) Bundle metrics
Bundle analysis report Branch PupilTong:p/hw/opt-out-wasm Project dashboard Generated by RelativeCI Documentation Report issue |
React MTF Example#935 Bundle Size — 196.68KiB (0%).5290d99(current) vs 5f3b6eb main#930(baseline) Bundle metrics
|
| Current #935 |
Baseline #930 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
174 |
174 |
|
66 |
66 |
|
44.05% |
44.05% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #935 |
Baseline #930 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
85.45KiB |
85.45KiB |
Bundle analysis report Branch PupilTong:p/hw/opt-out-wasm Project dashboard
Generated by RelativeCI Documentation Report issue
ef964b5 to
4b09aa2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.changeset/enable-web-binary-template-by-default.md (1)
18-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWording "could support" still present — not fully addressed from previous round.
Line 18 still reads grammatically incorrect: "could support the new output format".
📝 Proposed fix
-**Upgrade to `@lynx-js/web-core@0.20.2` could support the new output format** +**Upgrade to `@lynx-js/web-core@0.20.2` to support the new output format**🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/enable-web-binary-template-by-default.md at line 18, The phrase "could support the new output format" is grammatically weak; update the sentence containing "could support the new output format" to a definitive form such as "supports the new output format" or "now supports the new output format" so the changelog reads clearly and confidently; locate the literal string "could support the new output format" in .changeset/enable-web-binary-template-by-default.md and replace it with the chosen corrected wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/enable-web-binary-template-by-default.md:
- Line 2: The changeset currently marks "@lynx-js/template-webpack-plugin" as
"patch" despite a behavioral default change (feat) that flips output from JSON
to binary; update the changeset entry by changing the version bump for
"@lynx-js/template-webpack-plugin" from "patch" to "minor" so the release semver
reflects a non-patch behavioral change and aligns with the commit subject.
---
Duplicate comments:
In @.changeset/enable-web-binary-template-by-default.md:
- Line 18: The phrase "could support the new output format" is grammatically
weak; update the sentence containing "could support the new output format" to a
definitive form such as "supports the new output format" or "now supports the
new output format" so the changelog reads clearly and confidently; locate the
literal string "could support the new output format" in
.changeset/enable-web-binary-template-by-default.md and replace it with the
chosen corrected wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15172207-f3a2-410e-b1de-8c370a33389b
📒 Files selected for processing (4)
.changeset/enable-web-binary-template-by-default.mdpackages/web-platform/web-core-e2e/scripts/generate-build-command.jspackages/web-platform/web-tests/package.jsonpackages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts
💤 Files with no reviewable changes (1)
- packages/web-platform/web-core-e2e/scripts/generate-build-command.js
React Example with Element Template#69 Bundle Size — 198.12KiB (0%).5290d99(current) vs 5f3b6eb main#64(baseline) Bundle metrics
Bundle size by type
|
| Current #69 |
Baseline #64 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
52.36KiB |
52.36KiB |
Bundle analysis report Branch PupilTong:p/hw/opt-out-wasm Project dashboard
Generated by RelativeCI Documentation Report issue
4b09aa2 to
93c37b9
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.changeset/enable-web-binary-template-by-default.md (2)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBump should likely be
minor, notpatch.This changeset flips the default web template encoding from JSON to Binary — a user-observable behavior change for every consumer that doesn't set
EXPERIMENTAL_USE_WEB_BINARY_TEMPLATE. SemVer-wise that's a new feature/behavior change and warrants aminorbump rather thanpatch, especially given thefeat(web):subject.📝 Proposed change
-"@lynx-js/template-webpack-plugin": patch +"@lynx-js/template-webpack-plugin": minor🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/enable-web-binary-template-by-default.md at line 2, The changeset currently marks "@lynx-js/template-webpack-plugin" as a patch but the change flips default web template encoding (user-visible) so update the changeset to use a minor bump instead of patch; in the .changeset entry that contains the line "@lynx-js/template-webpack-plugin": patch change "patch" to "minor" and ensure the changeset title/summary reflects the feat(web): behavior change so the release tooling will produce a minor release.
18-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAwkward phrasing in upgrade note.
"could support" reads oddly here; it's an instruction to upgrade so the new format is supported. Minor wording fix:
📝 Proposed fix
-**Upgrade to `@lynx-js/web-core@0.20.2` could support the new output format** +**Upgrade to `@lynx-js/web-core@0.20.2` to support the new output format**🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/enable-web-binary-template-by-default.md at line 18, The upgrade note sentence "**Upgrade to `@lynx-js/web-core@0.20.2` could support the new output format**" is awkward; change that phrase to a clearer instruction such as "**Upgrade to `@lynx-js/web-core@0.20.2` to support the new output format**" or "**Upgrade to `@lynx-js/web-core@0.20.2` to enable support for the new output format**" by editing the same line in the changeset.
🧹 Nitpick comments (1)
packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts (1)
108-135: 💤 Low valueOpt-out check is now correctly string-only.
The conditional only matches
'false'/'0'(both validprocess.envstring values), and all other states — including unset — fall through to the binary path, which matches the new default documented in the changeset.One small consideration: the comparison is case-sensitive, so
EXPERIMENTAL_USE_WEB_BINARY_TEMPLATE=False(orFALSE) would silently take the binary path rather than the intended JSON opt-out. If you want to be lenient toward common user inputs:♻️ Optional case-insensitive opt-out
- const isExperimentalWebBinary = process - .env['EXPERIMENTAL_USE_WEB_BINARY_TEMPLATE']; - if ( - isExperimentalWebBinary === 'false' - || isExperimentalWebBinary === '0' - ) { + const isExperimentalWebBinary = process + .env['EXPERIMENTAL_USE_WEB_BINARY_TEMPLATE']?.toLowerCase(); + if ( + isExperimentalWebBinary === 'false' + || isExperimentalWebBinary === '0' + ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts` around lines 108 - 135, The opt-out check using process.env['EXPERIMENTAL_USE_WEB_BINARY_TEMPLATE'] is case-sensitive so values like "False" or "FALSE" will incorrectly enable the binary path; update the check around isExperimentalWebBinary in WebEncodePlugin.ts to perform a case-insensitive comparison (e.g. normalize via toLowerCase()) and guard for undefined/null so that only explicit "false" or "0" (in any case) choose the JSON branch; keep existing behavior for unset values to fall through to the binary path and reference the isExperimentalWebBinary variable and the surrounding return branches (JSON branch that uses genStyleInfo/tasmJSONInfo and the binary branch that imports encode).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In @.changeset/enable-web-binary-template-by-default.md:
- Line 2: The changeset currently marks "@lynx-js/template-webpack-plugin" as a
patch but the change flips default web template encoding (user-visible) so
update the changeset to use a minor bump instead of patch; in the .changeset
entry that contains the line "@lynx-js/template-webpack-plugin": patch change
"patch" to "minor" and ensure the changeset title/summary reflects the
feat(web): behavior change so the release tooling will produce a minor release.
- Line 18: The upgrade note sentence "**Upgrade to `@lynx-js/web-core@0.20.2`
could support the new output format**" is awkward; change that phrase to a
clearer instruction such as "**Upgrade to `@lynx-js/web-core@0.20.2` to support
the new output format**" or "**Upgrade to `@lynx-js/web-core@0.20.2` to enable
support for the new output format**" by editing the same line in the changeset.
---
Nitpick comments:
In `@packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts`:
- Around line 108-135: The opt-out check using
process.env['EXPERIMENTAL_USE_WEB_BINARY_TEMPLATE'] is case-sensitive so values
like "False" or "FALSE" will incorrectly enable the binary path; update the
check around isExperimentalWebBinary in WebEncodePlugin.ts to perform a
case-insensitive comparison (e.g. normalize via toLowerCase()) and guard for
undefined/null so that only explicit "false" or "0" (in any case) choose the
JSON branch; keep existing behavior for unset values to fall through to the
binary path and reference the isExperimentalWebBinary variable and the
surrounding return branches (JSON branch that uses genStyleInfo/tasmJSONInfo and
the binary branch that imports encode).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b3892b72-e2ef-44fb-8ef6-876e42badd79
📒 Files selected for processing (4)
.changeset/enable-web-binary-template-by-default.mdpackages/web-platform/web-core-e2e/scripts/generate-build-command.jspackages/web-platform/web-tests/package.jsonpackages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts
💤 Files with no reviewable changes (1)
- packages/web-platform/web-core-e2e/scripts/generate-build-command.js
Signed-off-by: Haoyang Wang <12288479+PupilTong@users.noreply.github.com>
Signed-off-by: Haoyang Wang <12288479+PupilTong@users.noreply.github.com>
93c37b9 to
8431c73
Compare
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/autolink-codegen@0.1.0 ### Minor Changes - Add the Native Autolink codegen package. ([#2601](#2601)) ## create-lynx-extension@0.1.0 ### Minor Changes - Add the Native Autolink create-extension package. ([#2587](#2587)) ### Patch Changes - Use published package versions for scaffolded autolink codegen dependencies instead of workspace placeholders. ([#2628](#2628)) - Fix npm bin symlink entrypoint detection for the create extension CLI. ([#2623](#2623)) ## @lynx-js/react@0.121.0 ### Minor Changes - Support `React.createElement(type, props, children)` API. ([#2360](#2360)) ```jsx React.createElement("view", { style }, <text>hello</text>); // equivalent to <view style={style}> <text>hello</text> </view>; React.createElement(MyComponent, { style }, <view />); // equivalent to <MyComponent style={style}> <view /> </MyComponent>; ``` ### Patch Changes - Clear transient snapshot child props when removed snapshot subtrees are detached, preventing compiled `$*` child references from retaining deleted list holder or list item subtrees after removal. ([#2590](#2590)) - Add `createPortal` for rendering a subtree into a different ReactLynx element identified by a `NodesRef`. ([#2543](#2543)) ```tsx function App() { const [host, setHost] = useState(null); return ( <view> <view ref={setHost} /> {host && createPortal(<text>hi</text>, host)} </view> ); } ``` - Default `fireEvent` to `bubbles: true` for the TouchEvent family in testing-library to match Lynx runtime semantics, and stop reassigning the read-only `Event.prototype` accessors which threw `TypeError` in strict mode. ([#2532](#2532)) - Set `bundle-url` on lazy bundle border elements. ([#2537](#2537)) - Stop warning when `runWorklet` receives an invalid or missing main-thread function object. Invalid worklet contexts are still ignored, but nullish handler values no longer produce noisy `MainThreadFunction: Invalid function object` console output. ([#2586](#2586)) - Retain main-thread worklet context references before offscreen snapshot elements are materialized, so event, ref, gesture, and spread callbacks stay alive until the DOM update path can attach them. ([#2592](#2592)) - Update the @lynx-js/tasm dependency to 0.0.39 and align React template attribute descriptors with it. ([#2643](#2643)) - Avoid retaining transformed nested worklet contexts after worklet transformation. ([#2591](#2591)) Nested worklets transformed by the worklet runtime now keep their context recovery metadata through a weak reference, preventing cached transformed worklet functions from keeping list-item worklet contexts alive. ## @lynx-js/docs-mcp-server@0.2.3 ### Patch Changes - fix(docs-mcp): recursively crawl and register nested llms.txt resources ([#2317](#2317)) ## @lynx-js/rspeedy@0.14.4 ### Patch Changes - feat(qrcode): support get entry from api exposed from rspeedy.env.entries ([#2551](#2551)) - Updated dependencies \[[`ad1f90f`](ad1f90f)]: - @lynx-js/chunk-loading-webpack-plugin@0.3.4 - @lynx-js/web-rsbuild-server-middleware@0.20.4 - @lynx-js/cache-events-webpack-plugin@0.0.3 ## @lynx-js/lynx-bundle-rslib-config@0.3.3 ### Patch Changes - Update the @lynx-js/tasm dependency to 0.0.39 and align React template attribute descriptors with it. ([#2643](#2643)) ## @lynx-js/qrcode-rsbuild-plugin@0.4.7 ### Patch Changes - feat(qrcode): support get entry from api exposed from rspeedy.env.entries ([#2551](#2551)) ## @lynx-js/react-rsbuild-plugin@0.16.2 ### Patch Changes - Updated dependencies \[[`3e627b3`](3e627b3), [`7b8d63c`](7b8d63c), [`13a0776`](13a0776), [`a973c54`](a973c54), [`353b1b7`](353b1b7)]: - @lynx-js/template-webpack-plugin@0.11.1 - @lynx-js/react-refresh-webpack-plugin@0.3.6 - @lynx-js/react-alias-rsbuild-plugin@0.16.2 - @lynx-js/use-sync-external-store@1.5.0 - @lynx-js/react-webpack-plugin@0.9.2 - @lynx-js/css-extract-webpack-plugin@0.7.1 ## @lynx-js/web-core@0.20.4 ### Patch Changes - Always clone touch event lists when creating cross-thread events so synthetic touch events only carry structured-clone-safe primitive fields. ([#2636](#2636)) - Conditionally pass Card and Component params based on cardType in background thread. ([#2610](#2610)) - Add bidirectional decode worker heartbreak liveness messages. ([#2599](#2599)) - Add web support for the `<frame>` element by mapping it to `<lynx-view>`. ([#2604](#2604)) - Stop redeclaring `fetch` as a chunk-scope binding. Reusing the host ([#2562](#2562)) `window.fetch` from BTS chunks (instead of capturing the no-op stub the chunk wrapper used to install) lets the renderer issue real network requests. - Updated dependencies \[[`c1db603`](c1db603)]: - @lynx-js/web-elements@0.12.2 - @lynx-js/web-worker-rpc@0.20.4 ## @lynx-js/web-elements@0.12.2 ### Patch Changes - fix: xmarkdown create img incorrectly ([#2540](#2540)) ## @lynx-js/chunk-loading-webpack-plugin@0.3.4 ### Patch Changes - Override `__webpack_require__.e` so a single sync-then chunk load (the ([#2597](#2597)) typical lazy bundle case) bypasses `Promise.all`. It will make first screen in main thread can load lazy bundle synchronously when using dynamic import. ## @lynx-js/react-refresh-webpack-plugin@0.3.6 ### Patch Changes - Widen `@lynx-js/react-webpack-plugin` peer range to include `^0.9.0`. ([#2626](#2626)) ## @lynx-js/template-webpack-plugin@0.11.1 ### Patch Changes - feat(web): enable web binary template by default ([#2545](#2545)) The default encoding format for the web platform template has been changed from JSON to Binary. **Benefits for developers:** - **Smaller output size:** Binary templates are more compact than JSON strings, reducing the final bundle size. - **Faster load performance:** Binary templates parse faster than JSON in the runtime, improving the time-to-interactive for web applications. **How to turn off this feature:** If you encounter any issues with the new binary template format, you can revert to the previous JSON format by setting the environment variable `EXPERIMENTAL_USE_WEB_BINARY_TEMPLATE` to `'false'` or `'0'` before running your build commands. For example: `EXPERIMENTAL_USE_WEB_BINARY_TEMPLATE=false rspeedy build` **Upgrade to `@lynx-js/web-core@0.20.2` could support the new output format** See [`@lynx-js/web-core` Changelog](https://lynx-stack.dev/changelog/lynx-js--web-core) - Run TASM template encoding in a shared `tinypool` worker pool so multi-entry builds encode in parallel and watch-mode rebuilds reuse warm workers. ([#2634](#2634)) - Make `LynxTemplatePlugin.getLynxTemplatePluginHooks` a cross-module singleton so duplicate copies of this package (e.g. from npm hoist conflicts) share the same hooks per compilation. ([#2624](#2624)) - Update the @lynx-js/tasm dependency to 0.0.39 and align React template attribute descriptors with it. ([#2643](#2643)) - Updated dependencies \[[`ee79eff`](ee79eff), [`ded4de9`](ded4de9), [`cf01e94`](cf01e94), [`b989c1c`](b989c1c), [`8417e68`](8417e68)]: - @lynx-js/web-core@0.20.4 ## @lynx-js/react-umd@0.121.0 ## create-rspeedy@0.14.4 ## @lynx-js/react-alias-rsbuild-plugin@0.16.2 ## upgrade-rspeedy@0.14.4 ## @lynx-js/web-rsbuild-server-middleware@0.20.4 ## @lynx-js/web-worker-rpc@0.20.4 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Configuration
Compatibility
Chores / Tests
Documentation
Checklist